-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: validate function call json schemas for openai #1156
Conversation
bless you 🙌🏾 |
|
if prop.is_object() | ||
&& (prop.get("type").and_then(|t| t.as_str()) == Some("object")) | ||
{ | ||
ensure_valid_json_schema(prop); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could put a recursive counter here, but I don't think extensions are going to have massively nested json schemas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I would hope they wouldn't!
if let Some(properties_obj) = properties.as_object_mut() { | ||
for (_key, prop) in properties_obj.iter_mut() { | ||
if prop.is_object() | ||
&& (prop.get("type").and_then(|t| t.as_str()) == Some("object")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to set default to true like line 277?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh very nice!
I wonder if we need similar thing for gemini too!
Co-authored-by: angiejones <jones.angie@gmail.com>
Users reporting openai incompatibility with certain extensions.
Fixes #933, related to #945
Extensions are not providing valid json schemas for openai function tool calling. Adds a function to validate the schema and add missing json schema fields
Databricks likely handled the missing fields on their end.
Tests: